Add support for SLH-DSA by padelsbach · Pull Request #512 · wolfSSL/wolfHSM

padelsbach · 2026-08-14T19:54:19Z

No description provided.

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #512

Scan targets checked: wolfhsm-core-bugs, wolfhsm-crypto-bugs, wolfhsm-src

Findings: 11
11 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Reported findings require changes before merge.

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #512

Scan targets checked: wolfhsm-core-bugs, wolfhsm-crypto-bugs, wolfhsm-src

Findings: 7
7 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Reported findings require changes before merge.

Frauschi

Went through the SLH-DSA callback support. Big addition, and the structure mirrors the ML-DSA paths closely enough to follow easily.

Comments inline. The ones I would most like to see addressed are the DMA READ_POST after a denied READ_PRE in _HandleSlhDsaVerifyDma, the unvalidated seedSz / 3 in both keygen handlers, and the seed left resident in the comm packet. The rest are smaller consistency points against the ML-DSA and ML-KEM siblings, and the last few are nits you can take or leave.

Nothing here is a blocker.

Frauschi

Two small follow-ups from the last round - neither is blocking, and the rest of what I raised looks good now.

Copilot

Copilot review overview

🟡 Changes recommended

Cache-mode flag handling, DER error reporting, and DMA cleanup can currently produce false success or leave unintended cached keys.

Review effort: Balanced
Findings: 3

Open (3)

EPHEMERAL flag discards generated key in cache-only API · New
Explicit key IDs can leave cached keys after DMA failure · New
Zero-length DER serialization reported as successful · New

What changed in this PR

Adds end-to-end SLH-DSA support across wolfHSM, including key management, signing, verification, DMA, crypto callbacks, examples, and CI coverage.

Changes:

Adds SLH-DSA wire protocols and client/server cryptographic operations.
Supports cached, exported, seeded, non-DMA, DMA, and callback-only workflows.
Extends configuration, tests, examples, and CI for SLH-DSA.

File	Description
`wolfhsm/wh_settings.h`	Documents SLH-DSA buffer requirements.
`wolfhsm/wh_server_crypto.h`	Declares server cache helpers.
`wolfhsm/wh_message_crypto.h`	Defines SLH-DSA wire messages.
`wolfhsm/wh_crypto.h`	Declares DER serialization support.
`wolfhsm/wh_common.h`	Adds the SLH-DSA algorithm identifier.
`wolfhsm/wh_client_crypto.h`	Exposes client SLH-DSA APIs.
`test/wh_test_check_struct_padding.c`	Covers new message layouts.
`test/config/user_settings.h`	Enables SLH-DSA in tests.
`test-refactor/wh_test_list.c`	Registers SLH-DSA tests.
`test-refactor/posix/Makefile`	Adds verify-only configuration.
`test-refactor/misc/wh_test_check_struct_padding.c`	Covers refactored message layouts.
`src/wh_server_keystore.c`	Supports public-key export.
`src/wh_server_crypto.c`	Implements server operations and DMA handlers.
`src/wh_message_crypto.c`	Implements message translation.
`src/wh_crypto.c`	Implements SLH-DSA DER conversion.
`src/wh_client_cryptocb.c`	Integrates wolfCrypt callbacks.
`src/wh_client_crypto.c`	Implements client and DMA APIs.
`examples/posix/wh_posix_server/user_settings.h`	Enables server-side SLH-DSA.
`examples/posix/wh_posix_client/user_settings.h`	Enables client and callback-only modes.
`examples/posix/wh_posix_client/Makefile`	Adds callback-only build selection.
`examples/demo/client/wh_demo_client_crypto.h`	Declares the SLH-DSA demo.
`examples/demo/client/wh_demo_client_crypto.c`	Implements the SLH-DSA demo.
`examples/demo/client/wh_demo_client_all.c`	Runs the SLH-DSA demo.
`.github/workflows/build-and-test-refactor.yml`	Tests verify-only builds.
`.github/workflows/build-and-run-examples.yml`	Tests callback-only operation.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot

Copilot review overview

🟡 Changes recommended

Non-DMA responses expose uninitialized padding, and the server header lacks the required SLH-DSA type include.

Review effort: Balanced
Findings: 2

Open (2)

Uninitialized response padding leaks stack data · New
Missing SLH-DSA header include causes unknown type · New

Resolved since last review (3)

Zero-length DER serialization reported as successful
Explicit key IDs can leave cached keys after DMA failure
EPHEMERAL flag discards generated key in cache-only API

bigbrett · 2026-10-05T20:07:50Z

@padelsbach can you check the copilot comments and address if necessary, otherwise resolve and reassign me plz

bigbrett

Some nits and a concern about the silent fallback. Also need to update PQC algo list in docs to include SLH-DSA. Also needs rebase and conflicts resolved. Perhaps squash the commits as well since you are rebasing anyway.

bigbrett · 2026-10-30T21:51:39Z

      if: matrix.group == 'pq-dma'
      run: cd test-refactor/posix && make clean && make -j DMA=1 ASAN=1 MLDSA_VERIFY_ONLY=1 WOLFSSL_DIR=../../wolfssl && make run

+    # Verify-only drops the signing APIs, so this catches code that assumes them.


Suggested change

# Verify-only drops the signing APIs, so this catches code that assumes them.

bigbrett · 2026-10-30T21:54:41Z

+    word32    sigLen = sizeof(signature);
+    byte      addRnd[WC_SLHDSA_MAX_SEED];
+
+    memset(addRnd, 0x42, sizeof(addRnd));


The answer to the ultimate question. But perhaps also a comment saying "placeholder for user-supplied random input"

Added comment

bigbrett · 2026-10-30T21:55:52Z

 endif

+ifeq ($(SLHDSA_CB_ONLY),1)
+DEF += -DWH_CFG_SLHDSA_CB_ONLY


where did WH_CFG_xxx come from? Our wolfHSM config macros are spelled WOLFHSM_CFG_XXXX

Ha, good catch. Fixed

bigbrett · 2026-10-30T22:00:04Z

+                do {
+                    ret = wh_Client_RecvResponse(ctx, &group, &action, &res_len,
+                                                 WOLFHSM_CFG_COMM_DATA_LEN,
+                                                 (uint8_t*)dataPtr);
+                } while (ret == WH_ERROR_NOTREADY);


OK for a first stab, but we will eventually need async split transaction Request/response functions for all of these in the client API. Then the blocking one can just send the request and block on the response. wolfCrypt cryptocallback will then just use the blocking API. See how all the other crypto algorithms are implemetned in client API.

Def need a follow up PR with this. Fine for now.

Async split is a big change. Punting on that for now

bigbrett · 2026-10-30T22:08:27Z

+    if ((ret == WH_ERROR_REQUEST_SIZE) && _PqcSigCanVerifyLocally(type, key)) {
+        ret = CRYPTOCB_UNAVAILABLE;
+    }
+


I don't like this. Seems yet another place where we can silently fall back to local crypto on the client side. IMO this is a bad pattern. We dont want any surprises. IMO if the user explicitly requests something to go the HSM it should fail when it doesn't. There are already some edge cases in teh current code and I don't want to add more. Any reason you did it this way?

I did this on the other PR too, trying to get too fancy. Removed.

padelsbach force-pushed the add-crypto-cb-slhdsa branch 2 times, most recently from aed3238 to 7fe4069 Compare August 21, 2026 23:20

padelsbach force-pushed the add-crypto-cb-slhdsa branch 4 times, most recently from 64514ae to a9eb8a9 Compare September 16, 2026 19:16

padelsbach marked this pull request as ready for review September 16, 2026 19:49

padelsbach requested review from Frauschi, bigbrett and wolfSSL-Fenrir-bot and removed request for bigbrett September 16, 2026 19:49

padelsbach force-pushed the add-crypto-cb-slhdsa branch from a9eb8a9 to 43e9f7f Compare September 16, 2026 19:55

wolfSSL-Fenrir-bot requested changes Sep 16, 2026

View reviewed changes

padelsbach requested a review from wolfSSL-Fenrir-bot September 16, 2026 21:22

wolfSSL-Fenrir-bot requested changes Sep 16, 2026

View reviewed changes

padelsbach requested a review from wolfSSL-Fenrir-bot September 18, 2026 00:56

padelsbach assigned bigbrett Sep 18, 2026

Frauschi reviewed Sep 21, 2026

View reviewed changes

Frauschi assigned padelsbach Sep 21, 2026

bigbrett removed their assignment Sep 21, 2026

padelsbach assigned Frauschi Sep 22, 2026

padelsbach requested a review from Frauschi September 22, 2026 03:28

Frauschi reviewed Sep 22, 2026

View reviewed changes

Comment thread src/wh_server_crypto.c Resolved

Comment thread src/wh_client_crypto.c Resolved

Frauschi removed their assignment Sep 22, 2026

padelsbach assigned Frauschi Sep 22, 2026

Frauschi previously approved these changes Sep 23, 2026

View reviewed changes

Frauschi assigned bigbrett and unassigned Frauschi Sep 23, 2026

padelsbach changed the title ~~Add crypto callback mode for SLH-DSA~~ Add support for for SLH-DSA Sep 25, 2026

Copilot AI balanced review requested due to automatic review settings October 2, 2026 20:18

padelsbach dismissed Frauschi’s stale review via 69839ec October 2, 2026 20:18

padelsbach force-pushed the add-crypto-cb-slhdsa branch from aedd93b to 69839ec Compare October 2, 2026 20:18

Copilot started reviewing on behalf of padelsbach October 2, 2026 20:18

View session

Copilot AI reviewed Oct 2, 2026

View reviewed changes

Comment thread src/wh_client_crypto.c Resolved

Comment thread src/wh_client_crypto.c Resolved

Comment thread src/wh_crypto.c Resolved

padelsbach requested review from Frauschi and a balanced review from Copilot October 3, 2026 00:42

Copilot started reviewing on behalf of padelsbach October 3, 2026 00:53

View session

padelsbach removed their assignment Oct 3, 2026

Copilot AI reviewed Oct 3, 2026

View reviewed changes

Comment thread src/wh_server_crypto.c Resolved

Comment thread wolfhsm/wh_server_crypto.h Resolved

bigbrett assigned padelsbach and unassigned bigbrett Oct 5, 2026

padelsbach force-pushed the add-crypto-cb-slhdsa branch 2 times, most recently from bbc5a63 to aa14855 Compare October 9, 2026 16:06

padelsbach force-pushed the add-crypto-cb-slhdsa branch from fe43259 to 9efa82f Compare October 13, 2026 04:40

padelsbach force-pushed the add-crypto-cb-slhdsa branch from d408125 to 7531b59 Compare October 30, 2026 01:04

padelsbach assigned bigbrett Oct 30, 2026

bigbrett requested changes Oct 30, 2026

View reviewed changes

bigbrett assigned padelsbach and unassigned bigbrett and padelsbach Oct 30, 2026

padelsbach changed the title ~~Add support for for SLH-DSA~~ Add support for SLH-DSA Nov 13, 2026

padelsbach force-pushed the add-crypto-cb-slhdsa branch 2 times, most recently from c863daf to 352ff0e Compare November 13, 2026 23:38

Add SLH-DSA support

db1f170

padelsbach force-pushed the add-crypto-cb-slhdsa branch from 352ff0e to db1f170 Compare November 14, 2026 01:39

padelsbach assigned bigbrett and unassigned padelsbach Nov 14, 2026

Conversation

padelsbach commented Aug 14, 2026

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #512

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #512

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Frauschi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Frauschi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

bigbrett commented Oct 5, 2026

Uh oh!

bigbrett left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!